Update NIR <> snnTorch #246
Conversation
|
after some (way too much) time, this PR is ready for review! @jeshraghian @ahenkes1 can you advise on next steps? add tests? docs? how can I help? cheers! |
| @@ -141,6 +141,7 @@ def __init__( | |||
| output=False, | |||
| graded_spikes_factor=1.0, | |||
| learn_graded_spikes_factor=False, | |||
| reset_delay=True, | |||
There was a problem hiding this comment.
as a clarifying comment: @jeshraghian and I discussed this a while ago - this flag is needed to match up the neuron dynamics with NIR. (with the default value, snntorch will behave exactly as before)
There was a problem hiding this comment.
The latest version of the Leaky neuron had some minor refactoring to get it compatible with torch.compile(), so there will be some conflicts. I'll iron these out today.
|
This is wonderful, thanks @stevenabreu7 ! docs In terms of docs, we should generate docstrings that describe the arguments of those two import/export functions with a minimalist example. Any caveats can be added as bullet points in the docstrings, e.g., types of neurons supported, layers, neuron arguments supported. I can create a skeleton, though would you be able to fill the meat? Once we have that, I can also put together some ipynb stuff based on these docstrings. tests I'm not too sure how we can do unit tests for the actual nir-dependent stuff, though... any ideas? Could it be a matter of having a minimalist set of models, passing them in, making sure their outputs have the arguments (just an assertion the arguments exist. Checking its values is overkill and left to nir). Thoughts? |
|
sounds good! I added docstrings, lmk if they're okay like that. Also added suggestions for how to test in |
|
Alright, looking great @stevenabreu7! Some of the syntax from the leaky neuron was modified in another PR to make it compatible with Your suggestions for tests look great, as do the docstrings. |
|
thank you @jeshraghian! added test and some notes on the rleaky that's not currently supported. I can make a new PR for this, wouldn't take long, but can we merge already so that it runs smoothly for the NIR demo tomorrow? (otherwise it's a bit of a pain to use this branch of snntorch for the demo) |
|
added an issue for RLeaky support - will update the NIR support in a fresh PR in the near future |
|
Just merged! The demo will still need to be from the latest version that is installed from the source rather than pip installing. Though I can do a version increment in the morning, if that helps. Lmk. |
This PR will:
For RNNs to work, make sure to use the right version of NIRTorch (current PR: neuromorphs/NIRTorch#12)